Skip to content

fix: fix value bit corruption in right_shift#348

Merged
apoelstra merged 2 commits intoBlockstreamResearch:masterfrom
stringhandler:fix/value_padding
Mar 3, 2026
Merged

fix: fix value bit corruption in right_shift#348
apoelstra merged 2 commits intoBlockstreamResearch:masterfrom
stringhandler:fix/value_padding

Conversation

@stringhandler
Copy link
Copy Markdown
Contributor

@stringhandler stringhandler commented Feb 26, 2026 •
edited by apoelstra
Loading

Yet another option to solve #337

The tests are actually generated by Claude. The comments are a bit wordy, so happy to try clean it up if it bothers anyone.

The interesting thing to note is that the bug is not as a result of from_padded_bits, as apoelstra (and myself) suspected. Upon adding the two regression tests from #339, prune_regression_337_1 still fails, indicating another bug in from_padded_bits.

However, prune_regression_337_2 passes on this PR without any changes to from_padded_bits. The tests added in this PR also reproduce the error without calling from_padded_bits.

I have also tried to keep the optimization of not cloning the array if the bit is already equal to new_bit and added it to the true case as well.

For those interested here are the before and after benchmarks. Nothing jumps out at me as a huge improvement or degradation.

BEFORE (Commit f84b09d)

test node::redeem::benches::decode_fixed_program          ... bench:      46,840.83 ns/iter (+/- 596.58)
test value::benches::bench_value_create_512_256           ... bench:      60,676.67 ns/iter (+/- 285.67)
test value::benches::bench_value_create_512_256_compact   ... bench:       3,226.49 ns/iter (+/- 73.52)
test value::benches::bench_value_create_512_256_padded    ... bench:       3,277.02 ns/iter (+/- 108.64)
test value::benches::bench_value_create_64k               ... bench:     669,047.50 ns/iter (+/- 4,790.50)
test value::benches::bench_value_create_64k_compact       ... bench:      34,772.54 ns/iter (+/- 3,966.75)
test value::benches::bench_value_create_64k_padded        ... bench:      33,474.00 ns/iter (+/- 3,886.75)
test value::benches::bench_value_create_deep_some         ... bench:     402,400.62 ns/iter (+/- 3,995.75)
test value::benches::bench_value_create_deep_some_compact ... bench:     145,885.00 ns/iter (+/- 1,446.33)
test value::benches::bench_value_create_deep_some_padded  ... bench:       1,224.16 ns/iter (+/- 4.50)
test value::benches::bench_value_create_u2048             ... bench:     469,603.75 ns/iter (+/- 6,750.75)
test value::benches::bench_value_create_u2048_compact     ... bench:       2,117.00 ns/iter (+/- 7.90)
test value::benches::bench_value_create_u2048_padded      ... bench:       2,063.22 ns/iter (+/- 11.30)
test value::benches::bench_value_create_u64               ... bench:          22.49 ns/iter (+/- 0.32)
test value::benches::bench_value_create_u64_compact       ... bench:         165.02 ns/iter (+/- 1.33)
test value::benches::bench_value_create_u64_padded        ... bench:         114.52 ns/iter (+/- 0.88)
test value::benches::bench_value_display_512_256          ... bench:      69,926.25 ns/iter (+/- 804.88)
test value::benches::bench_value_display_64k              ... bench:     707,050.00 ns/iter (+/- 10,249.00)
test value::benches::bench_value_display_deep_some        ... bench:      44,677.50 ns/iter (+/- 727.31)
test value::benches::bench_value_display_u2024            ... bench:      44,629.52 ns/iter (+/- 341.24)
test value::benches::bench_value_display_u64              ... bench:         351.55 ns/iter (+/- 2.40)

AFTER (c70e3ed)

test node::redeem::benches::decode_fixed_program          ... bench:      47,575.83 ns/iter (+/- 1,888.92)
test value::benches::bench_value_create_512_256           ... bench:      62,040.00 ns/iter (+/- 684.18)
test value::benches::bench_value_create_512_256_compact   ... bench:       3,091.83 ns/iter (+/- 19.26)
test value::benches::bench_value_create_512_256_padded    ... bench:       3,034.38 ns/iter (+/- 17.38)
test value::benches::bench_value_create_64k               ... bench:     671,748.75 ns/iter (+/- 4,494.75)
test value::benches::bench_value_create_64k_compact       ... bench:      32,111.70 ns/iter (+/- 5,517.22)
test value::benches::bench_value_create_64k_padded        ... bench:      32,177.62 ns/iter (+/- 7,199.71)
test value::benches::bench_value_create_deep_some         ... bench:     401,901.88 ns/iter (+/- 4,945.75)
test value::benches::bench_value_create_deep_some_compact ... bench:     145,748.00 ns/iter (+/- 833.20)
test value::benches::bench_value_create_deep_some_padded  ... bench:       1,205.44 ns/iter (+/- 5.89)
test value::benches::bench_value_create_u2048             ... bench:     482,537.50 ns/iter (+/- 8,726.38)
test value::benches::bench_value_create_u2048_compact     ... bench:       2,169.61 ns/iter (+/- 7.86)
test value::benches::bench_value_create_u2048_padded      ... bench:       2,075.53 ns/iter (+/- 16.34)
test value::benches::bench_value_create_u64               ... bench:          32.03 ns/iter (+/- 0.91)
test value::benches::bench_value_create_u64_compact       ... bench:         162.57 ns/iter (+/- 1.75)
test value::benches::bench_value_create_u64_padded        ... bench:         113.74 ns/iter (+/- 0.67)
test value::benches::bench_value_display_512_256          ... bench:      74,847.14 ns/iter (+/- 1,099.57)
test value::benches::bench_value_display_64k              ... bench:     709,160.00 ns/iter (+/- 12,038.00)
test value::benches::bench_value_display_deep_some        ... bench:      43,401.18 ns/iter (+/- 559.59)
test value::benches::bench_value_display_u2024            ... bench:      44,913.00 ns/iter (+/- 477.15)
test value::benches::bench_value_display_u64              ... bench:         357.24 ns/iter (+/- 7.69)

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apoelstra apoelstra apoelstra approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stringhandler @apoelstra